Skip to content

feat: add plotDir support and debug_detectormap.py script#390

Draft
wtgee wants to merge 19 commits intomasterfrom
tickets/PIPE2D-1834
Draft

feat: add plotDir support and debug_detectormap.py script#390
wtgee wants to merge 19 commits intomasterfrom
tickets/PIPE2D-1834

Conversation

@wtgee
Copy link
Copy Markdown
Member

@wtgee wtgee commented Apr 22, 2026

Add _showOrSavePlot() helper to FitDistortedDetectorMapTask that saves all open matplotlib figures to /[_N].png when debugInfo.plotDir is set, or falls back to plt.show() for interactive use. Replace all 7 bare plt.show() calls in the task with named _showOrSavePlot() calls.

Add bin.src/debug_detectormap.py: a ready-to-use lsstDebug override that enables all diagnostic plot flags. The output directory is controlled via DETECTORMAP_PLOT_DIR and the set of enabled flags via DETECTORMAP_DEBUG_FLAGS.

Add _showOrSavePlot() helper to FitDistortedDetectorMapTask that saves
all open matplotlib figures to <plotDir>/<name>[_N].png when
debugInfo.plotDir is set, or falls back to plt.show() for interactive
use. Replace all 7 bare plt.show() calls in the task with named
_showOrSavePlot() calls.

Add bin.src/debug_detectormap.py: a ready-to-use lsstDebug override
that enables all diagnostic plot flags. The output directory is
controlled via DETECTORMAP_PLOT_DIR and the set of enabled flags via
DETECTORMAP_DEBUG_FLAGS.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@PaulPrice
Copy link
Copy Markdown
Member

debug_detectormap.py is not an executable. Shall we put it in a new debug directory?

@wtgee
Copy link
Copy Markdown
Member Author

wtgee commented Apr 22, 2026

debug_detectormap.py is not an executable. Shall we put it in a new debug directory?

Yes, good call, thanks. I'm going to test this out and make sure it works first then will clean the PR. Would be nice to start utilizing the existing plots if easy enough.

@PaulPrice
Copy link
Copy Markdown
Member

👍
That's a good find -- I didn't know about that functionality.

wtgee and others added 18 commits April 21, 2026 18:25
result.soften is always a (xSoften, ySoften) tuple from calculateFitStatistics,
so the scalar comparison 'result.soften > 0' raises a TypeError. Unpack and
apply each component independently.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The --debug-info flag does not exist; pipetask --debug looks for a
debug.py on PYTHONPATH instead.  Replace the stale usage pattern with
a shell wrapper (run_fitDetectorMap_debug.sh) that:

  1. Creates a temporary directory on each invocation
  2. Writes a minimal debug.py that delegates to debug_detectormap.py
     via importlib (so DRP_STELLA_DIR is resolved at runtime)
  3. Prepends the temp dir to PYTHONPATH
  4. Execs: pipetask run --debug "$@"
  5. Cleans up the temp dir on exit via trap

debug_detectormap.py is kept as the actual implementation and its
docstring is updated to reference the wrapper script.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove the shell wrapper in favour of option 2: users copy or symlink
docs/debug_detectormap.py to a directory on their PYTHONPATH as debug.py,
then run pipetask with --debug.  The docstring now explains this setup
step and the DETECTORMAP_PLOT_DIR / DETECTORMAP_DEBUG_FLAGS env vars.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
python/ is already on PYTHONPATH so no profile edits are needed.
Add python/debug.py to .gitignore to prevent accidental commits.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Plots saved to plotDir now include the visit, arm and spectrograph in
their filenames to avoid collisions when processing multiple dataIds:

  baseResiduals-v123456_b1.png
  residuals-v123456_b1_0.png   (multi-figure case)

_plotSuffix is set at the top of run() from visitInfo.id and the arm/
spectrograph keys of dataId, and initialised to '' in __init__ as a
safe fallback.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
result.distortion is a Distortion, not a DetectorMap, so it has no
getNumFibers()/getFiberId(). Sample the grid fiberId values from the
unique fibers present in the lines data instead.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Distortion.__call__ takes (xBase, yBase) pixel coordinates, not
(fiberId, wavelength). The previous code was calling it with integer
fiber IDs and wavelength values which is an incompatible type.

Replace the fiber/wavelength meshgrid with a regular grid in the
distortion's own (xBase, yBase) domain (from distortion.getRange()),
drawing numGridLines iso-x and iso-y lines sampled at numSamples points
each. This correctly visualises how the distortion warps the base
pixel grid.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
getXDistortion()/getYDistortion()/getRightCcd() belong to DetectorDistortion;
MosaicPolynomialDistortion (used for b/r/m arms) instead exposes
getXPoly()/getYPoly()/getAffine(), so the old code always failed on non-n arms.

Rewrite to use only the common Distortion API (getRange() + __call__):
- Evaluate distortion(xBase, yBase) on a numSamples x numSamples grid
- Compute displacement dxModel = model_x - base_x, dyModel = model_y - base_y
- Show displacement images alongside observed residuals at arc line positions

Works for all Distortion subclasses.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When the same plot type is produced in multiple iterations (sigma-clipping
loop, trace iterations, slit-offset iterations), all files were saved with
the same name, each overwriting the last.

Add self._plotCallCounts (reset in run()) that increments each time
_showOrSavePlot() is called with a given name.  The counter is appended
before any per-figure index:

  <name>-v<visit>_<arm><spec>_<N>[_<M>].png

e.g. model-v121319_b1_0.png (sigma-clip iter 0)
     model-v121319_b1_3.png (final/softened fit)
     slitOffsets-v121319_b1_0.png .. _2.png (3 iterations)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
LayeredDetectorMap (and other DetectorMap subclasses) do not expose a
.model attribute -- that is specific to SplinedDetectorMap.  The
model.getScaling().dispersion call was only used to convert wavelength
residuals to pixels.  Replace it with detectorMap.getDispersionAtCenter(),
which is defined on the DetectorMap base class and works for all subclasses.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
legendHandles was renamed to legend_handles in matplotlib 3.5.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Instead of dumping all plots into a single flat directory, create:

    <plotDir>/<YYYYmmddTHHMM>/<arm><spec>-v<visit>/<name>_N.png

- The session timestamp (_PLOT_SESSION) is captured at module import time
  (minute precision) so all quanta spawned within the same pipetask run
  share the same top-level subdirectory.
- Each data ID gets its own subdirectory (<arm><spec>-v<visit>), removing
  the need to embed the data ID in each filename.
- The per-name call counter suffix is retained so iteration plots remain
  distinguishable (e.g. model_0.png ... model_3.png).

Also removes self._plotSuffix (replaced by self._plotSubdir).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add _saveQaStats() to FitDistortedDetectorMapTask: writes
  wavelengthStats_N.csv (per arc-line wavelength) and fiberStats_N.csv
  (per fiber) to the plot directory whenever DETECTORMAP_PLOT_DIR is set.
  Both files contain mean and robust-RMS x/y residuals plus line counts,
  making it straightforward to check for blue-wavelength spatial biases
  without needing DEBUG-level logging.

- Add bin.src/parse_detectormap_log.py: standalone script that parses
  INFO-level fitDetectorMap log output into structured tables and optional
  CSV/plot files.  Extracts overall fit summary, per-species, per-fiber,
  and (if DEBUG was enabled) per-wavelength statistics.  With --qa-dir it
  also reads the wavelengthStats CSVs and produces a mean-x-residual vs
  wavelength plot for direct blue-bias diagnosis.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A line that is detected (finite y) on all fibers but has yErr=NaN for
every fiber is a strong indicator of saturation: the ISR SAT mask
removes the peak core, the Gaussian fitter finds an approximate center
but cannot estimate sigma, and the line is then silently dropped by the
finite-error check.  Emit a WARNING so the operator can see which lines
are affected without needing to inspect per-wavelength QA stats.

Example trigger: HgI 546.2268 nm (intensity ~80k, 6× brighter than the
next HgI line) on the blue arm.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The previous warning in getGoodLines() checked ArcLineResidualsSet.y
(residual dy) for finiteness, which may not reliably detect the
saturated-line signature depending on how centroid failures are handled.

Move the check to a new _warnSaturatedLines() method called from run()
before any residual computation, operating directly on the original
ArcLineSet centroids. This is unambiguous: lines.y is the raw centroid,
and lines.yErr is the sigma estimate that fails for saturated lines.

Also:
- Use self.log.warn() instead of self.log.warning() to match the form
  used elsewhere in the file (line 919)
- Replace the em-dash with '--' (pure ASCII) to avoid potential
  encoding issues in log handlers
- Remove the duplicate check from getGoodLines()

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The previous approach of checking yErr=NaN for all fibers was incorrect:
np.median() returns NaN if even a single value is NaN, so 'mean error=nan'
in lineQa does not imply ALL yErr are NaN. Some fibers may still have
finite yErr, causing np.any(isfinite(yErr)) to be True and short-circuit
the check.

The definitive indicator that a line is completely dropped by getGoodLines
is flag=True for every fiber. Switch to checking this directly:
- warn when all fibers (>= 10) have flag=True for a wavelength
- log the description so the user can distinguish saturation (bright
  mid-range lines like HgI 546.2 nm) from non-detection (faint/edge lines)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ySelection in calculateFitStatistics did not filter lines with NaN xErr/yErr,
so NaN errors could reach calculateSoftening even when upstream 'use' masks
required finite errors. This only manifests for specific detectors (e.g. b2)
where some lines have finite centroids but NaN measurement errors.

Two defenses:
1. Add np.isfinite(lines.xErr/yErr) to xSelection/ySelection so NaN errors
   are excluded from the softening calculation.
2. Guard the softenChi2(0.0) evaluation before calling scipy.optimize.bisect,
   since NaN is neither < 0 nor > 0, causing both early-exit branches to be
   skipped and bisect to fail with ValueError.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace _saveQaStats (file-based, required DETECTORMAP_PLOT_DIR) with
  _logQaStats, which emits one INFO line per arc-line wavelength in a
  structured QA_WL format.  No env var is required; QA stats are now
  always present in every fitDetectorMap log.

- parse_detectormap_log.py now parses QA_WL lines and writes a single
  aggregated wavelength_qa.csv (arm/spectrograph/visit/wavelength/…)
  when --output-dir is given.  The run key now includes the visit so
  logs from multiple visits/detectors are kept separate and aggregated
  correctly.

- Drop _PLOT_SESSION / import datetime.  Plot directory structure
  changes from <plotDir>/<YYYYmmddTHHMM>/<arm><spec>-v<visit>/ to
  <plotDir>/<arm><spec>-v<visit>/, eliminating the bookkeeping burden
  when many visits span multiple minutes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants